-
Notifications
You must be signed in to change notification settings - Fork 841
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[EuiPanel] Expanding usages for layout purposes #4194
Conversation
- changed `backgroundColor` to just `color` - color and radius modifiers are now in the mixin
'euiCard--shadow', // For matching EuiPanel mixin | ||
'euiCard--borderRadiusMedium', // For matching EuiPanel mixin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a temporary fix/hack until the next step which will be using EuiPanel directly.
'euiCheckableCard--shadow', // For matching EuiPanel mixin | ||
'euiCheckableCard--borderRadiusMedium', // For matching EuiPanel mixin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
'transparent': transparent, | ||
'plain': $euiColorEmptyShade, | ||
'subdued': $euiColorLightestShade, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only supporting these colors in the default theme because the others don't quite work well with the border.
Preview documentation changes for this PR: https://eui.elastic.co/pr_4194/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 🎉 Tested on Chrome, Firefox, Safari, and Edge.
I think it makes sense to have the ability to turn off borderRadius
and the shadow
. This will allow using this component to create many more types of layouts. I totally support the deprecations.
I just added a few suggestions.
Co-authored-by: Elizabet Oliveira <elizabet.oliveira@elastic.co>
Preview documentation changes for this PR: https://eui.elastic.co/pr_4194/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes LGTM; viewed & tested docs changes.
Preview documentation changes for this PR: https://eui.elastic.co/pr_4194/ |
* [EuiPanel] Added `color` and `borderRadius` options * [EuiPanel] `hasShadow` defaults to `true` but is the slight version - color and radius modifiers are now in the mixin * Fix up usages * Updated docs and added deprecation notices
I struggled with the title on this one.
Essentially, the aim here was to promote EuiPanel not just as a bordered box, but as a layout container consumers can use to simply provide padding and/or shading to areas of their pages.
This is also the first step towards being able to reuse the component itself in more higher-order components instead of the Sass mixin which creates way more selectors. For this reason, there may look to be some inefficiencies in the SASS but, it's the easiest fix for now.
🔔 The goal
➕ Additions
1. Added
color
propThis prop is mainly in preparation for more colored usages by the Amsterdam theme. Currently the default theme will only support plain, subdued, and transparent. While the Amsterdam theme will support all of these:
This is always a step towards reusing EuiPanel and these colors for use in EuiCallout.
2. Added
borderRadius
to turn off rounded cornersThe options are simply 'm' or 'none'.
🛠 Updates
3. Changed
hasShadow
to betrue
by default butfalse
now fully removes the shadowBefore, EuiPanel always has some sort of a shadow where
hasShadow
just increased it. More and more we're seeing the need to remove the shadow not increase it. I checked through Kibana and there aren't really any usages of increasing the shadow. So this should be a safe assumption.☠️ Set
onClick
and all beta badge props for deprecationThis was originally applied before we decided to go down the EuiCard path. EuiCard handles the interaction behavior and accessibility of interaction way better (even at all) than EuiPanel. Since EuiCard can now also contain any content, let's let that higher-order component be the one to allow interaction. Similar reasoning behind the beta badge deprecations.
Most of the rest of the changes are updating downstream usages and fixing up styles to better support the default props. As well as updating docs.
Checklist
[ ] Checked for breaking changes and labeled appropriately[ ] Checked for accessibility including keyboard-only and screenreader modes